feat(autoware_lanelet_map_validator): add dangling reference checker to non existing intersection_area #177
Conversation
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
soblin
force-pushed
the
feat/vm-03-08-dangling-reference-intersection-area
branch
from
adae44c to
7e22a32
Compare
…to non existing intersection_area Signed-off-by: Mamoru Sobue <[email protected]>
soblin
force-pushed
the
feat/vm-03-08-dangling-reference-intersection-area
branch
from
7e22a32 to
60ea165
Compare
soblin
requested review from
anhnv3991,
Motsu-san,
SakodaShintaro,
TaikiYamada4 and
YamatoAndo
as code owners
There was a problem hiding this comment.
Thank you for the great work!
I wrote my opinions below so please check them out.
Aside those comments
- Could you write a brief document to explain what this validator can do?
- Could you add this to
autoware_requirement_set.jsonlike the following?
{
"id": "vm-03-01",
"validators": [
{
"name": "mapping.intersection.intersection_area_dangling_reference"
}
]
},| lanelet::validation::Severity::Error, lanelet::validation::Primitive::Lanelet, lanelet.id(), | ||
| append_issue_code_prefix( | ||
| this->name(), 1, | ||
| "Lanelet of ID " + std::to_string(lanelet.id()) + |
There was a problem hiding this comment.
The primitive (lanelet) and its ID will be displayed automatically since they are already given in L83, so the message starting with "This lanelet" might be fine.
| if (lanelet.attributeOr("turn_direction", "none") == std::string("none")) { | ||
| return std::nullopt; | ||
| } |
There was a problem hiding this comment.
Is this part necessary?
There was a problem hiding this comment.
I think it is not good for a non-intersection lane to have "intersection_area" entry cause it's meaningless. And current implementation ignores that anyway. But I think we should sanitize that.
There was a problem hiding this comment.
I think that the order
- Collect all intersection_area polygons first
- Scan through lanelets and find defected lanelets using
is_intersection_with_area.
will be much efficient that we don't need to collect intersection_with_area_lanelets, but what do you think?
| class TestIntersectionAreaDanglingReferenceError : public MapValidationTester | ||
| { | ||
| protected: | ||
| void SetUp() override | ||
| { | ||
| // prepare `map_` and `loading_errors_` | ||
| // this class uses erroneous map | ||
| load_target_map("intersection/intersection_area_with_dangling_reference.osm"); | ||
| } | ||
|
|
||
| private: | ||
| }; | ||
|
|
||
| class TestIntersectionAreaDanglingReferenceOK : public MapValidationTester | ||
| { | ||
| protected: | ||
| void SetUp() override | ||
| { | ||
| // prepare `map_` and `loading_errors_` | ||
| // this class uses valid map | ||
| load_target_map("intersection/basic_intersection_area.osm"); | ||
| } | ||
|
|
||
| private: | ||
| }; |
There was a problem hiding this comment.
I think only one class is enough so try loading the maps at the start of the test functions if there's no special reason.
You can write the validation subject on the second argument of TEST_F.
Description
This PR aims to add checker if any intersection lanelets (namely with
turn_directiontag) has valid VALUE as theintersection_areakey. If there are no intersection_area with corresponding VALUE id, it is dangling reference.How was this PR tested?
Added a test map "intersection_area_with_dangling_reference.osm" with:
Notes for reviewers
None.
Effects on system behavior
None.